HSEARCH-5464 Pluggable rest clients in the elasticsearch backend #4798

marko-bekhta · 2025-09-09T12:16:18Z

https://hibernate.atlassian.net/browse/HSEARCH-5464

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

since the behavior of this client is slightly different

…plement the clients as dependencies

marko-bekhta

Hey @yrodiere @lucamolteni 🙂 👋🏻

I've added some comments here and there in this PR in places that may be of interest.
Otherwise there's a lot of "noise", from moving packages and renaming.

This PR doesn't include the docs+migration guide updates. I was thinking about sending that separately, as this already has a lot of changes 🙂.

marko-bekhta · 2025-10-16T07:38:53Z

...java/org/hibernate/search/backend/elasticsearch/client/common/spi/ElasticsearchResponse.java

 	private final JsonObject body;

-	public ElasticsearchResponse(HttpHost host, int statusCode, String statusMessage, JsonObject body) {
+	public ElasticsearchResponse(String host, int statusCode, String statusMessage, JsonObject body) {


This one is pretty straightforward, we only use the host to log the response, and it's an SPI ... so removing the apache class here should be "safe"

marko-bekhta · 2025-10-16T07:42:11Z

...ain/java/org/hibernate/search/backend/elasticsearch/client/common/gson/spi/GsonProvider.java

+	 * https://github.com/google/gson/issues/764 is supposedly fixed.
+	 * Hence, we may not need the workaround anymore.
+	 * This version will be used in the test so we'll notice if things are still broken, for the main code
+	 * the workaround is in place through the GsonProviderHelper.


This GSON issue was fixed, but I kept the workaround for the main code "just in case".

marko-bekhta · 2025-10-16T07:46:48Z

...bernate/search/backend/elasticsearch/client/ElasticsearchHttpClientConfigurationContext.java

+	@Deprecated(since = "8.2", forRemoval = true)
+	default Optional<ElasticsearchVersion> configuredVersion() {
+        return ConfigurationProperty.forKey(ElasticsearchBackendSettings.VERSION)
+                .as(ElasticsearchVersion.class, ElasticsearchVersion::of)
+                .build().get(configurationPropertySource());
+	}


since we are pushing the rest client code to a different jar, we cannot keep using ElasticsearchVersion hence the deprecation... (otherwise we'd need to push the ElasticsearchVersion somewhere else so it would accessible by both client and backend jars ... and pushing it to the client common didn't feel right)

marko-bekhta · 2025-10-16T07:57:27Z

...tiontest/backend/elasticsearch/client/ElasticsearchClientFactoryImplElasticsearchJavaIT.java

+		wireMockRule1.stubFor( post( urlPathMatching( "/myIndex/myType/_search" ) )
+				.withRequestBody( equalToJson( payload ) )
+				.willReturn( elasticsearchResponse().withStatus( 401 )
+						.withHeader( "www-authenticate", "Basic realm=\"IT Realm\"" ) ) );
+


this one might be interesting to know 🙂 elasticsearch-java client does not support "preemptive authentication" so it doesn' send the credentials right away, and it was causing the test to fail as the client never receied a 401 to send credentials.

marko-bekhta · 2025-10-16T07:59:38Z

...te/search/backend/elasticsearch/client/java/ElasticsearchHttpClientConfigurationContext.java

+@SuppressJQAssistant(
+		reason = "Apache HTTP Client 5 uses a lot of classes/interfaces in the impl packages to create builders/instances etc. "
+				+ "So while it is bad to expose impl types ... in this case it's what Apache Client expects users to do?")
 public interface ElasticsearchHttpClientConfigurationContext {


More of an FYI 🙈

org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder

marko-bekhta · 2025-10-16T08:07:23Z

...search/backend/elasticsearch/client/common/cfg/ElasticsearchBackendClientCommonSettings.java

-	 * <p>
-	 * Defaults to no request timeout.
-	 */
-	public static final String REQUEST_TIMEOUT = "request_timeout";


the idea here is to push any more-or-less client specific config properties to corresponding client impl modules.
with the apache based clients these are duplicated, but that might not be the case with other impls. So better stay safe 🙂

marko-bekhta · 2025-10-16T08:34:20Z

...e/search/backend/elasticsearch/client/common/spi/ElasticsearchRequestInterceptorContext.java

+
+	Map<String, String> queryParameters();
+
+	void overrideHeaders(Map<String, List<String>> headers);


for aws signing I just reshuffled the existing code to remove any apache specifics and push them to the client impls. Since we just need to set a few headers that signing might produce, this seems pretty straightforward.

marko-bekhta · 2025-10-16T08:37:27Z

...ibernate/search/backend/elasticsearch/client/java/impl/ClientJavaHttpRequestInterceptor.java

+			if ( entity instanceof AsyncEntityProducer producer ) {
+				if ( !producer.isRepeatable() ) {
+					throw new AssertionFailure( "Cannot sign AWS requests with non-repeatable entities" );
+				}
+				return new HttpAsyncEntityProducerInputStream( producer, 1024 );
+			}


This one is a bit annoying as there seems to be no easier/simpler way to get the entity out of the request, but well ... it is what it is 🙂 🤷🏻

marko-bekhta · 2025-10-16T08:45:09Z

...n/java/org/hibernate/search/backend/elasticsearch/client/opensearch/impl/GsonHttpEntity.java

+ *
+ * @author Sanne Grinovero (C) 2017 Red Hat Inc.
+ */
+final class GsonHttpEntity implements HttpEntity, AsyncEntityProducer {


the apache client 5 apis changed a bit so these gson entities were updated a bit 🙂
the tests are passing and all seems good, but if anyone would want to have an extra look, that would be nice 🙂

marko-bekhta · 2025-10-16T08:53:57Z

...ava/org/hibernate/search/backend/elasticsearch/client/ElasticsearchHttpClientConfigurer.java

+@SuppressWarnings("removal")
+@Deprecated(since = "8.2", forRemoval = true)
+public interface ElasticsearchHttpClientConfigurer
+		extends org.hibernate.search.backend.elasticsearch.client.rest.ElasticsearchHttpClientConfigurer {

 	/**
 	 * Configure the HTTP Client.
 	 * <p>
 	 * This method is called once for every configurer, each time an Elasticsearch client is set up.
 	 * <p>
 	 * Implementors should take care of only applying configuration if relevant:
 	 * there may be multiple, conflicting configurers in the path, so implementors should first check
 	 * (through a configuration property) whether they are needed or not before applying any modification.
 	 * For example an authentication configurer could decide not to do anything if no username is provided,
 	 * or if the configuration property {@code my.configurer.enabled} is {@code false}.
 	 *
 	 * @param context A configuration context giving access to the Apache HTTP client builder
 	 * and configuration properties in particular.
 	 */
 	void configure(ElasticsearchHttpClientConfigurationContext context);

+	@Override
+	default void configure(
+			org.hibernate.search.backend.elasticsearch.client.rest.ElasticsearchHttpClientConfigurationContext context) {
+		configure( new ElasticsearchHttpClientConfigurationContextDelegate( context ) );
+	}


Here's how I went about making existing configurers work when the Search code internally only calls the "new configurer" method.

sonarqubecloud · 2025-10-16T22:34:05Z

Quality Gate failed

Failed conditions
78.6% Coverage on New Code (required ≥ 80%)
25.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch 7 times, most recently from aac26ce to 1d41060 Compare September 10, 2025 14:31

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch 3 times, most recently from 3da3726 to 610c554 Compare September 16, 2025 21:10

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch from 610c554 to 343b198 Compare October 13, 2025 21:12

marko-bekhta added 13 commits October 16, 2025 10:01

HSEARCH-5464 Remove the apache host from ElasticsearchResponse

d180b9c

HSEARCH-5464 Move out client common classes

b36d223

HSEARCH-5464 Remove dependency on ES version class in the client code

c40f74d

HSEARCH-5464 Continue factoring out the Client

1bebda0

HSEARCH-5464 OpenSearch client

737968b

HSEARCH-5464 Elasticsearch-java client

8c417aa

HSEARCH-5464 Test different clients

a394f2b

HSEARCH-5464 Rename the clients

2efc2d6

HSEARCH-5464 Adjust elasticsearch-java tests

a95dc9e

since the behavior of this client is slightly different

HSEARCH-5464 Fix aggregated javadocs and dependency alignment

501f827

HSEARCH-5464 More renames to address JQAssistant rules

6e58e32

HSEARCH-5464 Explicitly list the apache http client libs we use to im…

0e54837

…plement the clients as dependencies

HSEARCH-5464 Push client specific settings to client modules

57ed41f

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch from 1a49649 to e91fd98 Compare October 16, 2025 08:02

marko-bekhta added 3 commits October 16, 2025 10:23

HSEARCH-5464 Make AWS signing compatible with other client impls

3792aba

HSEARCH-5464 Adjust search-util-common exports

2801e79

HSEARCH-5464 Add missing platform dependencies

ae10272

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch from e91fd98 to ae10272 Compare October 16, 2025 08:29

marko-bekhta commented Oct 16, 2025

View reviewed changes

HSEARCH-5464 Add a few notes on the new pluggable clients

2e6dca2

marko-bekhta force-pushed the feat/HSEARCH-5464-Groundwork-for-pluggable-rest-clients-in-the-Elasticsearch-backend branch from e347a5f to 2e6dca2 Compare October 16, 2025 20:30


		Map<String, String> queryParameters();

		void overrideHeaders(Map<String, List<String>> headers);

Uh oh!

HSEARCH-5464 Pluggable rest clients in the elasticsearch backend #4798

Are you sure you want to change the base?

HSEARCH-5464 Pluggable rest clients in the elasticsearch backend #4798

Uh oh!

Conversation

marko-bekhta commented Sep 9, 2025

Uh oh!

marko-bekhta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 16, 2025

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant